Conversation
| {% macro circleci_form(request, pending_circleci_publisher_form) %} | ||
| <p> | ||
| {% trans href="https://circleci.com/docs/openid-connect-tokens/" %} | ||
| Read more about CircleCI's OpenID Connect support <a href="{{ href }}">here</a>. |
There was a problem hiding this comment.
todo: prep page about this form for CCI guides, lets add a link to that as well. though its a chicken egg issue i suspect. but if we can get this deployed to staging warehouse, can then build a guide and do a followup pr to add the link before hitting prod
warehouse/migrations/versions/549bdbefa6bd_add_circleci_oidc_publisher.py
Outdated
Show resolved
Hide resolved
warehouse/migrations/versions/549bdbefa6bd_add_circleci_oidc_publisher.py
Outdated
Show resolved
Hide resolved
a2568a4 to
44d7223
Compare
|
milestone: the 0.0.1.dev87 was published from to my local pypi
|
6ec2327 to
9fe9b93
Compare
5b980b7 to
459fa6e
Compare
459fa6e to
669a67a
Compare
1e6acfb to
bdae7b2
Compare
|
So I think this is ready for review. I left some open questions about process - eg: would it be preferred for me to add in the attestation work into this pr? or better to leave stacked. |
di
left a comment
There was a problem hiding this comment.
FYI, looks like translations need updated and also the "revises" field for the database migration as we've had additional migrations since you generated it.
bdae7b2 to
92ccbca
Compare
|
converted to draft while I add the api lookup for org name and project name, as well as fix up one or two other things |
Add tests for parity with GitHub/GitLab form tests: - ProjectNameUnavailable*Error variants (Invalid, Existing, Stdlib, Prohibited, Similar) - Optional field consistency tests for vcs_ref, vcs_origin, context_id - Parametrized invalid field tests for required UUID fields - Empty string vs None consistency for optional fields Signed-off-by: meeech <4623+meeech@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019c003a-2977-70f9-ad05-cce7fb3236a0 Co-authored-by: Amp <amp@ampcode.com>
Fill in actual values for optional CircleCI fields (context_id, vcs_ref, vcs_origin) to match the pattern used by GitHub/GitLab tests which populate their optional fields with real values. Signed-off-by: meeech <4623+meeech@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-019c003a-2977-70f9-ad05-cce7fb3236a0 Co-authored-by: Amp <amp@ampcode.com>
Add pipeline_definition_id, context_id, vcs_ref, and vcs_origin to the CircleCI trusted publisher display in manage_base.html. Also update test factories to include vcs_ref and vcs_origin fields. Amp-Thread-ID: https://ampcode.com/threads/T-019c2939-421d-757c-9608-2f7b1758b9c0 Co-authored-by: Amp <amp@ampcode.com>
ran make resetdb and make initdb, seems fine
align handling optional fields to how other publishers handle it. was doing or "" in a few places - but we can see that github normalizes the env on the form itself, and then use the normalized field where necessary
* update schema for new metadata we fetch from cci api - org name, proj name * remove nullable=True for the optionals on the publisher - we normalize these to empty strings since they are part of the unique constraints - they should never be null
Amp-Thread-ID: https://ampcode.com/threads/T-019cac83-e053-704b-ab1f-39513bcdce9a Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019cac83-e053-704b-ab1f-39513bcdce9a Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019cac83-e053-704b-ab1f-39513bcdce9a Co-authored-by: Amp <amp@ampcode.com>
- Refactor publisher_base_url and publisher_url methods to return constructed URLs instead of None. - Update tests to reflect the new URL structure. - Modify template to link project name and ID to their respective CircleCI URLs.
f54ede0 to
de7dcf9
Compare
303a370 to
749a665
Compare
|
@di I think this is ready for another review when y'all are. |
|
@di I think I've addressed all the feedback and its ready again |
|
Hey @meeech, thanks for your work here so far. I'm just a volunteer on the project and as a result have had less time to spend working on it lately. I've asked @miketheman who is PSF staff to review instead. |
|
@di thank you for your work on this :D Sorry about that - i wasn't sure about who to ping - just figured since you were the latest to give feedback I did. trying to be mindful of not making demands on peeps/giving time for reviews since i know a lot of people are busy. def appreciate the feeedback so far, and thanks for pinging @miketheman |
|
@miketheman anything i can do to move this forward? |


#13888
Related PRs:
pypi/pypi-attestations#166 Add CircleCI to pypi-attestations
meeech#1 Stacked pr off this one - integrates attestations. Question: would it be better to just merge this into have the one pr? I was trying to be mindful of making things easier to review.
I have tried to keep commits bite sized. I am open to split this up into multiple prs if that preferred (though not sure where the 'fault lines' would be for this since its all a bit related - forms, model, etc? let me know
MUST
while i worked with a LLM to generate this PR, i (the human) have reviewed all this code. (amp for the curious)
tbf it did a good job doing all the boilerplate.
Basically I try to adhere to https://github.com/ghostty-org/ghostty/blob/main/AI_POLICY.md